7105350: HttpExchange's attributes are the same as HttpContext's attributes #27652

SentryMan · 2025-10-06T16:09:40Z

Now ExchangeImpl will default to having a separate attribute map for the request duration.

Progress

Change must be properly reviewed (1 review required, with at least 1 Reviewer)
Change must not contain extraneous whitespace
Commit message must refer to an issue
Change requires a CSR request matching fixVersion 26 to be approved (needs to be created)

Issue

JDK-7105350: HttpExchange's attributes are the same as HttpContext's attributes (Bug - P4)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/27652/head:pull/27652
$ git checkout pull/27652

Update a local copy of the PR:
$ git checkout pull/27652
$ git pull https://git.openjdk.org/jdk.git pull/27652/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 27652

View PR using the GUI difftool:
$ git pr show -t 27652

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/27652.diff

Using Webrev

Link to Webrev Comment

bridgekeeper · 2025-10-06T16:10:54Z

👋 Welcome back SentryMan! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

openjdk · 2025-10-06T16:11:41Z

❗ This change is not yet ready to be integrated.
See the Progress checklist in the description for automated requirements.

openjdk · 2025-10-06T16:12:43Z

@SentryMan The following label will be automatically applied to this pull request:

net

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

mlbridge · 2025-10-06T16:15:55Z

Webrevs

Michael-Mc-Mahon

Probably need a test for this as well

Michael-Mc-Mahon · 2025-10-07T09:22:39Z

src/jdk.httpserver/share/classes/sun/net/httpserver/ExchangeImpl.java

+    private static final boolean perExchangeAttributes =
+        !System.getProperty("jdk.httpserver.attributes", "")
+              .equals("context");


The property will have to be documented somewhere such as the jdk.httpserver module-info. But, I'm wondering if fairly obscure properties like this might be better off "buried" in the net.properties config file. I think there's a case for putting all such "compatibility" flags somewhere out of the way like that. If we were to do that, you would need to access the property using sun.net.NetProperties.

Any other views on this?

I can switch to sun.net.NetProperties if you so desire.

Yeah, I think we should put the property in net.properties something like this:

# Prior to JDK 26, the HttpExchange attribute map was shared with the enclosing HttpContext. # Since JDK 26, by default, exchange attributes are per-exchange and the context attributes must # be accessed by calling getHttpContext().getAttributes(). Uncomment this property to # restore the pre JDK 26 behavior. # # jdk.httpserver.attributes=context

Then I think we should add a sentence at the end of the jdk.httpserver module-info to say that additional
system/networking properties may be defined in net.properties

That proposal sounds good to me. Documenting the compatibiity in net.properties makes sense.

src/jdk.httpserver/share/classes/sun/net/httpserver/ExchangeImpl.java

dfuch

I agree with @Michael-Mc-Mahon that we will need a test to verify both old (with property set) and new behavior. Also this change will require a CSR and release notes.

Otherwise the proposed solution looks good. I believe it meets what a user reading the documentation would expect: get/set attributes on context get/set attributes for the whole context and may be observed by all exchanges that operate on this context. get/set attribute on the exchange get/set attributes whose scope is limited to that particular exchange.

This was a long standing issue and I'm happy to see it fixed. Thanks for driving that @SentryMan and @Michael-Mc-Mahon !

dfuch · 2025-10-08T12:53:48Z

src/jdk.httpserver/share/classes/sun/net/httpserver/ExchangeImpl.java

+    private static final boolean perExchangeAttributes =
+        !System.getProperty("jdk.httpserver.attributes", "")
+              .equals("context");


That proposal sounds good to me. Documenting the compatibiity in net.properties makes sense.

openjdk · 2025-10-08T14:40:22Z

@SentryMan core-libs has been added to this pull request based on files touched in new commit(s).

dfuch · 2025-10-08T14:41:06Z

/csr needed

openjdk · 2025-10-08T14:42:47Z

@dfuch has indicated that a compatibility and specification (CSR) request is needed for this pull request.

@SentryMan please create a CSR request for issue JDK-7105350 with the correct fix version. This pull request cannot be integrated until the CSR request is approved.

src/jdk.httpserver/share/classes/module-info.java

SentryMan · 2025-10-08T14:52:59Z

@SentryMan please create a CSR request for issue JDK-7105350 with the correct fix version.

How should I go about creating a CSR? The FAQ on this page says that I need JIRA access (I don't have that)

dfuch · 2025-10-08T14:55:23Z

@SentryMan note: WRT to creating a CSR your sponsor will handle that if you do not have a JBS account. Let's wait until this PR has been approved and everyone is happy with the API doc / behaviours changes.

AlanBateman · 2025-10-08T15:02:02Z

src/java.base/share/classes/module-info.java

        java.net.http,
-        jdk.naming.dns;
+        jdk.naming.dns,
+        jdk.httpserver;


It would be good if we could avoid giving the jdk.httpserver module access to the sun.net.* package.

@dfuch @Michael-Mc-Mahon Are you sure that net.properties is the right place for this?

@AlanBateman I hadn't realised that the HttpServer didn't already have access to sun.net.* - and I agree that it would be better if we did not add such an access. That said - the new property is a compatibility property - and documenting in module-info seemed a bit too much - so out of the way in net.properties seemed like an appealing place to add it.

Another possibility could be to document it only in the release notes.

Shall I revert the change and leave as a system property?

SentryMan added 2 commits October 6, 2025 11:47

Separate attributes for exchangeImpl

3b3715f

Merge branch 'openjdk:master' into JDK-7105350

a1f2873

openjdk bot added the net [email protected] label Oct 6, 2025

openjdk bot added the rfr Pull request is ready for review label Oct 6, 2025

Michael-Mc-Mahon reviewed Oct 7, 2025

View reviewed changes

require non null

62937b8

dfuch reviewed Oct 8, 2025

View reviewed changes

SentryMan added 2 commits October 8, 2025 10:09

net property

9d1d35b

Update module-info.java

434d2a9

openjdk bot added the core-libs [email protected] label Oct 8, 2025

openjdk bot added the csr Pull request needs approved CSR before integration label Oct 8, 2025

dfuch reviewed Oct 8, 2025

View reviewed changes

src/jdk.httpserver/share/classes/module-info.java Outdated Show resolved Hide resolved

Update module-info.java

7711d93

AlanBateman reviewed Oct 8, 2025

View reviewed changes

7105350: HttpExchange's attributes are the same as HttpContext's attributes #27652

Are you sure you want to change the base?

7105350: HttpExchange's attributes are the same as HttpContext's attributes #27652

Conversation

SentryMan commented Oct 6, 2025 • edited by openjdk bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Progress

Issue

Reviewing

Uh oh!

bridgekeeper bot commented Oct 6, 2025

Uh oh!

openjdk bot commented Oct 6, 2025

Uh oh!

openjdk bot commented Oct 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mlbridge bot commented Oct 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Webrevs

Uh oh!

Michael-Mc-Mahon left a comment

Choose a reason for hiding this comment

Uh oh!

Michael-Mc-Mahon Oct 7, 2025

Choose a reason for hiding this comment

Uh oh!

SentryMan Oct 8, 2025

Choose a reason for hiding this comment

Uh oh!

Michael-Mc-Mahon Oct 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dfuch Oct 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

dfuch left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dfuch Oct 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

openjdk bot commented Oct 8, 2025

Uh oh!

dfuch commented Oct 8, 2025

Uh oh!

openjdk bot commented Oct 8, 2025

Uh oh!

Uh oh!

SentryMan commented Oct 8, 2025

Uh oh!

dfuch commented Oct 8, 2025

Uh oh!

AlanBateman Oct 8, 2025

Choose a reason for hiding this comment

Uh oh!

dfuch Oct 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SentryMan Oct 8, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

SentryMan commented Oct 6, 2025 •

edited by openjdk bot

Loading

openjdk bot commented Oct 6, 2025 •

edited

Loading

mlbridge bot commented Oct 6, 2025 •

edited

Loading

Michael-Mc-Mahon Oct 8, 2025 •

edited

Loading

dfuch Oct 8, 2025 •

edited

Loading

dfuch left a comment •

edited

Loading

dfuch Oct 8, 2025 •

edited

Loading

dfuch Oct 8, 2025 •

edited

Loading